[SOU-301] Fix branch filtering to use short names instead of refs/heads#806
[SOU-301] Fix branch filtering to use short names instead of refs/heads#806
Conversation
WalkthroughRemoved a mapping step in the branch selection flow for indexing in repoIndexManager.ts. Matching branches are now appended to revisions as raw branch names instead of being converted to full refs/heads/ format, altering the composition of the revisions array. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@msukkari your pull request is missing a changelog! |
| allBranches | ||
| .filter((branch) => micromatch.isMatch(branch, branchGlobs)) | ||
| .map((branch) => `refs/heads/${branch}`); | ||
| .filter((branch) => micromatch.isMatch(branch, branchGlobs)); |
There was a problem hiding this comment.
I don't think this is a correct change. #800 is referring to the filter on the /repos page. We need to have refs/heads/ and refs/tags/ to allow the user to distinguish between filtering a branch and a tag (e.g., in the event where a tag and branch are named the same thing). This is documented here: https://docs.sourcebot.dev/docs/features/search/multi-branch-indexing#search-syntax
There was a problem hiding this comment.
The current suggested change would still be a welcomed change.
- Making sure
rev:maps to the short name of the default branch makes it easier for non technical people to understand which branch they are looking at. They might not know whatrefs/headsstands for, but they would know whatmain,feature/, etc are as that is generally what is displayed in all code hosting platforms - If you specify, which branches to index in the SB configs, like this
"revisions": {
"branches":
"main"
"release/*"
]
it indexes main and ref/head separately (iirc)
3. I am working on adding the feature to be able to switch revisions directly from the code tree/preview panels. and this would be useful in my efforts. (and goes hand in hand with 1. above)
There was a problem hiding this comment.
- Making sure rev: maps to the short name of the default branch makes it easier for non technical people to understand which branch they are looking at.
I believe zoekt does a substring match. For example:
I would like to keep it such that a user can explicitly specify a refs/heads or refs/tags like this. In the UI, I think we can just strip the refs/heads & refs/tags
2.If you specify, which branches to index in the SB configs, like this it indexes main and ref/head separately (iirc)
This actually appears to be a bug. Tracked in #808
- I am working on adding the feature to be able to switch revisions directly from the code tree/preview panels. and this would be useful in my efforts. (and goes hand in hand with 1. above)
Cool! I assume you mean something like a dropdown selector that lets users switch between branches? Created #810 to track this if you want to discuss over there.
|
Fixed in #851 |
3 participants
Problem
Searching on filtered branches was returning no results because branch names were being stored with the
refs/heads/prefix when they should have been stored as short names only.Solution
Remove the unnecessary mapping that prefixed branch names with
refs/heads/in the branch filtering logic. The zoekt index stores branch references as short names (e.g.,main,develop), so the filtering logic should match this format.Changes
.map((branch) => \refs/heads/${branch}`)from branch filtering inrepoIndexManager.ts`This fixes the issue where the
rev:query syntax was not finding any results when filtering on indexed branches.Fixes #800
View Niteshift Task
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.